Conversation
| try { | ||
| const apiUrl = buildApiUrl(request, settings); | ||
| const prerendered = await fetchPrerendered(apiUrl, request, settings); | ||
| settings.afterRender(request, prerendered); |
There was a problem hiding this comment.
I'd await this one as well and highlight that it should be a promise like for beforeRender.
| function mockFetch(status = 200, body = PRERENDERED_HTML) { | ||
| global.fetch = async () => ({ | ||
| status, | ||
| headers: new Headers({ 'content-type': 'text/html' }), | ||
| text: async () => body | ||
| }); | ||
| } |
There was a problem hiding this comment.
this has no teardown/restore between tests. If a previous test leaves a broken mock, the next test inherits it silently.
| function buildResponse(h, prerendered) { | ||
| const response = h.response(prerendered.body).code(prerendered.status).takeover(); | ||
| for (const [key, value] of prerendered.headers.entries()) { | ||
| response.header(key, value); | ||
| } | ||
| return response; | ||
| } |
There was a problem hiding this comment.
Should we skip forwarding headers that are managed by Hapi internally (e.g., content-length, content-encoding)? Apparently other integrations (Express middleware) typically do filter these — worth checking if this causes issues in practice with Hapi v21
something like
const SKIP_HEADERS = new Set(['content-encoding', 'content-length', 'transfer-encoding', 'connection']);
for (const [key, value] of prerendered.headers.entries()) {
if (!SKIP_HEADERS.has(key.toLowerCase())) response.header(key, value);
}
| "url": "git://github.com/prerender/integrations", | ||
| "directory": "prerender-hapi" |
There was a problem hiding this comment.
mmm shouldn't it be prerender/hapi?
| '.dmg', '.iso', '.flv', '.m4v', '.torrent', '.ttf', '.woff', '.svg' | ||
| ]; | ||
|
|
||
| internals.defaults = { |
There was a problem hiding this comment.
we could add jsdoc, it's just comments but helps as a very light type checker
/**
* @typedef {{ status: number, headers: Headers, body: string }} PrerenderResponse
*/
/**
* @typedef {Object} PrerenderOptions
* @property {string} [token]
* @property {string} [serviceUrl]
* @property {string|null} [protocol]
* @property {(request: import('@hapi/hapi').Request) => Promise<PrerenderResponse|null>} [beforeRender]
* @property {(request: import('@hapi/hapi').Request, response: PrerenderResponse) => void|Promise<void>} [afterRender]
*/
Also as suggested later I'd move the afterRender as a Promise
| return `${base}${protocol}://${request.headers.host}${pathname}${search}`; | ||
| } | ||
|
|
||
| async function fetchPrerendered(apiUrl, request, settings) { |
There was a problem hiding this comment.
jsdoc
/**
* @returns {Promise<PrerenderResponse>}
*/
|
|
||
| exports.plugin = { | ||
| pkg: require('./package.json'), | ||
| async register(server, options) { |
There was a problem hiding this comment.
jsdoc
/**
* @param {PrerenderOptions} options
*/
| if (settings.token) { | ||
| headers['X-Prerender-Token'] = settings.token; | ||
| } |
There was a problem hiding this comment.
maybe we can log a warning here, could be useful to debug for the client.
Initial implementation of the Prerender.io integration for hapi.